Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Make default slice view orientations configurable #5536

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented Mar 19, 2021

User can choose between radiology and neurology view directions (patient left/right on screen right).

@lassoan lassoan requested a review from pieper March 19, 2021 01:32
@lassoan lassoan self-assigned this Mar 19, 2021
@pieper
Copy link
Member

pieper commented Mar 19, 2021

I'm really torn on this. We have been very consistently using the radiological convention and it has allowed us to be unambiguous to answer orientation questions. Given the history of this issue, I think that before we enable this switch we should turn the orientation marker by default and even add L and R (and other letters or letter combinations for reformats) to the sides of the slice views so it is very explicit.

@lassoan
Copy link
Contributor Author

lassoan commented Mar 19, 2021

I agree that placing axis letters at top/bottom/left/right of the slice views (double or triple letters in case of oblique views) would be a more common and nicer way of representing orientations than the arrow/box/human figure, we should implement that at some point for sure. They are also somewhat more subtle than the orientation marker in the corner, so we could maybe enable orientation marker by default if we use this style.

I'm not sure if we need to enforce orientation marker display more if we expose this option. The option will only be found if someone looks for it and right next to it there is the option of showing the orientation marker. Also, someone can already use reformat module to flip slice orientation.

By the way, this is the post that triggered this work: https://twitter.com/gadevenyi/status/1372200767994597385

Most other image viewer software that I've tried makes it easy to flip views very easily. I wanted to check if it is easy to change the default in Slicer and it was not too hard. It just seemed unreasonable to ask users to always go to Reformat module and choose a view, then move a slider, then choose another view and move a slider to get patient-right-is-screen-right orientation.

If you feel very strongly about this then maybe I can hide this from application settings and force users to edit the Slicer.ini file, but I'm not sure if it helped that much overall.

@pieper
Copy link
Member

pieper commented Mar 19, 2021

This is sort of like nifti files, where the neuroscience community adopted their own convention and it kind of leaked out. It's definitely an issue in the clinical research world where both radiological and neurological software meet. I would not be surprised if there have been clinical consequences. I'm afraid that, for example, someone might send a slicer screenshot and the recipient wouldn't know that the setting had been changed on that slicer version. I'd feel more comfortable if we implemented the side letters before exposing this option. This would also help neuroscientist be aware when the images are displayed in radiological convention.

@lassoan
Copy link
Contributor Author

lassoan commented Mar 19, 2021

Adding a new orientation marker style would not mean that user would suddenly start using orientation markers. What do you think about automatically enabling the orientation marker if someone changes the slice view orientation? (we could use the arrow style for now and later could add the letters at the border)

@pieper
Copy link
Member

pieper commented Mar 19, 2021

What do you think about automatically enabling the orientation marker if someone changes the slice view orientation?

Yes, this would be good. I think the axis arrows are the most clear (human is too subtle).

@gdevenyi
Copy link

Origin of the twitter thread here ✋🏻

I'm okay with hiding it within the config file, and I'd be 👍🏻 for having orientation labels always be present. Coming from MINC-land to NIFTI-land I was surprised at all the possible ambiguities of orientation/display, depending on software and on-disk format (Slicer vs ITKsnap vs FSL(eyes,view) vs MRIcron, MRICrogl) and I agree there's a risk of someone changing the settings and it not being obvious when collaborating.

@lassoan lassoan force-pushed the configurable-slice-view-orientation branch from b203960 to 43fbc9d Compare March 22, 2021 01:06
@lassoan
Copy link
Contributor Author

lassoan commented Mar 22, 2021

@pieper Now if the user switches to the non-default view then orientation marker is turned on automatically.

User can choose between radiology and neurology view directions (patient left/right on screen right).
When the user chooses the non-default (patient right on
@lassoan lassoan force-pushed the configurable-slice-view-orientation branch from 43fbc9d to cb28eaf Compare March 22, 2021 01:34
Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding that, this makes me more comfortable with it.

@pieper pieper merged commit 69671bd into Slicer:master Mar 22, 2021
lassoan added a commit to lassoan/Slicer that referenced this pull request Apr 8, 2021
Default SliceToRAS transforms were recently changed so that they become configurable and at the same time they were also changed to be right-handed (see Slicer#5536).

The side-effect of enforcing right-handed coordinate systems was that the slice increment direction (which direction the slice moves when the right/up arrow key or forward wheel is moved) got reversed for two slice views (see https://discourse.slicer.org/t/inverted-slice-view-scrolling-direction/16977/8).

Making all coordinate systems right-handed is preferable in general (to avoid turning surfaces inside out, etc.), but in this case everything was already prepared to work with right-handed coordinate systems, so the easiest solution to fix slice scrolling directions is to revert the default SlicerToRAS coordinate systems to the previously used ones.
lassoan added a commit to lassoan/Slicer that referenced this pull request Apr 8, 2021
Default SliceToRAS transforms were recently changed so that they become configurable and at the same time they were also changed to be right-handed (see Slicer#5536).

The side-effect of enforcing right-handed coordinate systems was that the slice increment direction (which direction the slice moves when the right/up arrow key or forward wheel is moved) got reversed for two slice views (see https://discourse.slicer.org/t/inverted-slice-view-scrolling-direction/16977/8).

Making all coordinate systems right-handed is preferable in general (to avoid turning surfaces inside out, etc.), but in this case everything was already prepared to work with right-handed coordinate systems, so the easiest solution to fix slice scrolling directions is to revert the default SlicerToRAS coordinate systems to the previously used ones.
pieper pushed a commit that referenced this pull request Apr 8, 2021
* ENH: Added detailed error reporting to mrml file loading

* BUG: Fix crash after deleting a transform that had a 3D widget

This steps resulted in a crash: create new transform, show 3D widget, delete transform, click in a 3D view, hit any key.

The problem was that a keyboard interaction observer remained active after the transform widget was deleted that called method of the deleted widget.

Fixed by disabling the widget before deleting it. This removes all the observers properly.

* BUG: Restore slice increment/decrement direction in slice views

Default SliceToRAS transforms were recently changed so that they become configurable and at the same time they were also changed to be right-handed (see #5536).

The side-effect of enforcing right-handed coordinate systems was that the slice increment direction (which direction the slice moves when the right/up arrow key or forward wheel is moved) got reversed for two slice views (see https://discourse.slicer.org/t/inverted-slice-view-scrolling-direction/16977/8).

Making all coordinate systems right-handed is preferable in general (to avoid turning surfaces inside out, etc.), but in this case everything was already prepared to work with right-handed coordinate systems, so the easiest solution to fix slice scrolling directions is to revert the default SlicerToRAS coordinate systems to the previously used ones.
lassoan added a commit that referenced this pull request Apr 8, 2021
Default SliceToRAS transforms were recently changed so that they become configurable and at the same time they were also changed to be right-handed (see #5536).

The side-effect of enforcing right-handed coordinate systems was that the slice increment direction (which direction the slice moves when the right/up arrow key or forward wheel is moved) got reversed for two slice views (see https://discourse.slicer.org/t/inverted-slice-view-scrolling-direction/16977/8).

Making all coordinate systems right-handed is preferable in general (to avoid turning surfaces inside out, etc.), but in this case everything was already prepared to work with right-handed coordinate systems, so the easiest solution to fix slice scrolling directions is to revert the default SlicerToRAS coordinate systems to the previously used ones.
@lassoan lassoan deleted the configurable-slice-view-orientation branch April 8, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants